Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Jun 6, 2025

This PR removes the asyncio package, as it will probably need to change radically, so we don't want to ship it for 1.0.

It also generalizes logging.get_public_logger to be about module names (so getting the public part of a module name instead of limit it to only work with loggers) and makes UNIX_EPOCH final.

It also updates the release notes.

This package needs rethinking, so it is better not to ship it in
v1.0.0.

Signed-off-by: Leandro Lucarella <[email protected]>
Copilot AI review requested due to automatic review settings June 6, 2025 11:57
@llucax llucax requested a review from a team as a code owner June 6, 2025 11:57
@github-actions github-actions bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:asyncio Affects the asyncio module part:datetime Affects the datetime module labels Jun 6, 2025
@llucax llucax added this to the v1.0.0 milestone Jun 6, 2025
@llucax llucax self-assigned this Jun 6, 2025

This comment was marked as outdated.

@llucax llucax requested a review from shsms June 6, 2025 12:02
@github-actions github-actions bot added the part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) label Jun 6, 2025
@llucax llucax requested a review from Copilot June 6, 2025 12:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Prepare for the 1.0 release by stripping out the experimental asyncio package, generalizing the module-name helper, finalizing the epoch constant, and updating release notes and label configs

  • Remove all asyncio utilities and their tests to avoid shipping unstable code
  • Rename get_public_logger to get_public_module_name and adjust its implementation and tests
  • Annotate UNIX_EPOCH as a Final constant
  • Update RELEASE_NOTES.md and GitHub labeler configs with new components

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_module.py Updated import and assertions to use get_public_module_name
tests/asyncio/* Deleted all asyncio-related test files
src/frequenz/core/module.py Renamed helper to get_public_module_name and rewrote docstring
src/frequenz/core/datetime.py Marked UNIX_EPOCH as Final[datetime]
.github/labeler.yml / keylabeler.yml Added new part:module and part:id labels
RELEASE_NOTES.md Revised summary and component list
Comments suppressed due to low confidence (3)

tests/test_module.py:26

  • [nitpick] The test function name and its docstring still refer to get_public_logger—please rename them (e.g. test_get_public_module_name) to match the updated helper.
def test_get_public_logger(module_name: str, expected_logger_name: str) -> None:

RELEASE_NOTES.md:5

  • [nitpick] Consider mentioning the removal of the asyncio utilities in this summary to inform users about backward-incompatible changes in the 1.0 release.
This is the initial release of the Frequenz Core Library, which provides a set of fundamental tools and utilities for Python.

RELEASE_NOTES.md:10

  • [nitpick] List markers are inconsistent (mix of - and *). Please use a consistent bullet style throughout.
* `id`: For creating unique system-wide ID types.

llucax added 4 commits June 6, 2025 14:11
This module and function were too specific to `logging`, when it could
be more generally useful if it just deal with module names instead.

The module was renamed to `core.module` and the function to
`get_public_module_name()`, and it now deals only with strings.

When this is needed to get a logger, users can simply do:

logger = logging.getLogger(module.get_public_module_name(__name__))

Signed-off-by: Leandro Lucarella <[email protected]>
This ensures it can't be re-bound to some other value.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Copy link

@daniel-zullo-frequenz daniel-zullo-frequenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

But somehow I'm not fully convinced about removing all the functionality around asyncio. Maybe we can keep the util and task group modules if the service* is the problem?

@llucax
Copy link
Contributor Author

llucax commented Jun 6, 2025

But somehow I'm not fully convinced about removing all the functionality around asyncio. Maybe we can keep the util and task group modules if the service* is the problem?

Nah, we keep using what's in the SDK until we get it right. The code is there, we can always revert the commit if we want it back.

@llucax llucax added this pull request to the merge queue Jun 6, 2025
Merged via the queue into frequenz-floss:v1.x.x with commit bfa3ef3 Jun 6, 2025
5 checks passed
@llucax llucax deleted the prepare-1.0 branch June 6, 2025 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:asyncio Affects the asyncio module part:datetime Affects the datetime module part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants